Skip to content

Fix incorrect condition of noLib - #58867

Merged
Sheetal Nandi (sheetalkamat) merged 2 commits into
mainfrom
noLibCheck
Jun 18, 2024
Merged

Sheetal Nandi (sheetalkamat) merged 2 commits into
mainfrom
noLibCheck

Conversation

@sheetalkamat

Copy link
Copy Markdown
Member

No description provided.

@MichaelMitchell-at

Copy link
Copy Markdown
Contributor

Verified this fixes the issue described in #58863 (comment) ! Thank you!

@jakebailey

Copy link
Copy Markdown
Member

We should have run the extended tests on this; it this causes a crash in the vscode codebase, and also causes some 100 errors there too.

@jakebailey

Copy link
Copy Markdown
Member

I had to revert this in #58935; it caused compilation of vscode to crash when --diagnostics was passed, and some 100+ missing lib errors.

@sheetalkamat

Copy link
Copy Markdown
Member Author

I had to revert this in #58935; it caused compilation of vscode to crash when --diagnostics was passed, and some 100+ missing lib errors.

The errors dont seem to be because of this PR. I ran tsc -p src with and without fix and the number of errors didnt change.
I do see the crash and looking into whats going on. But this fix is partially correct.

@jakebailey

Copy link
Copy Markdown
Member

Oops, you're totally right. I think I was incorrectly comparing against another baseline. That being said, I don't know why they have the errors at all, https://github.com/microsoft/vscode/blob/main/src/tsconfig.base.json#L23 says ES2022 so we should also be pulling in ES2021.

@sheetalkamat

Copy link
Copy Markdown
Member Author

Its because one of their file c:/temp/vscode/src/vs/workbench/contrib/webview/browser/pre/service-worker.js has no-default-lib is true. I will revisit #57524 and come up with some proposal but thats the cause.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants